Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(ko): Enable the ko builder in the API #6820

Merged
merged 1 commit into from
Nov 10, 2021
Merged

feat(ko): Enable the ko builder in the API #6820

merged 1 commit into from
Nov 10, 2021

Conversation

halvards
Copy link
Collaborator

@halvards halvards commented Nov 9, 2021

Tracking: #6041
Obsolete: #6654

@halvards halvards requested a review from a team as a code owner November 9, 2021 22:36
@google-cla google-cla bot added the cla: yes label Nov 9, 2021
MarlonGamez
MarlonGamez requested changes Nov 9, 2021
View reviewed changes
Copy link
Contributor

@MarlonGamez MarlonGamez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not much to say here, looks good!

Can we add a little piece of code here to make sure that ko built artifacts get accounted for in our metadata events?

skaffold/pkg/skaffold/event/v2/metadata.go

Lines 83 to 111 in ea2b8b5

func getArtifacts(b latestV1.BuildConfig) []*proto.BuildMetadata_Artifact {
result := []*proto.BuildMetadata_Artifact{}
for _, a := range b.Artifacts {
artifact := &proto.BuildMetadata_Artifact{
Name: a.ImageName,
Context: a.Workspace,
}
switch {
case a.BazelArtifact != nil:
artifact.Type = proto.BuilderType_BAZEL
case a.BuildpackArtifact != nil:
artifact.Type = proto.BuilderType_BUILDPACKS
case a.CustomArtifact != nil:
artifact.Type = proto.BuilderType_CUSTOM
case a.DockerArtifact != nil:
artifact.Type = proto.BuilderType_DOCKER
artifact.Dockerfile = a.DockerArtifact.DockerfilePath
case a.JibArtifact != nil:
artifact.Type = proto.BuilderType_JIB
case a.KanikoArtifact != nil:
artifact.Type = proto.BuilderType_KANIKO
artifact.Dockerfile = a.KanikoArtifact.DockerfilePath
default:
artifact.Type = proto.BuilderType_UNKNOWN_BUILDER_TYPE
}
result = append(result, artifact)
}
return result
}

@codecov
Copy link

codecov bot commented Nov 9, 2021 •
edited
Loading

Codecov Report

Merging #6820 (8284714) into main (290280e) will decrease coverage by 1.22%.
The diff coverage is 64.65%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6820      +/-   ##
==========================================
- Coverage   70.48%   69.25%   -1.23%     
==========================================
  Files         515      546      +31     
  Lines       23150    24973    +1823     
==========================================
+ Hits        16317    17296     +979     
- Misses       5776     6517     +741     
- Partials     1057     1160     +103
Impacted Files Coverage Δ
cmd/skaffold/app/cmd/deploy.go 52.00% <ø> (-1.85%) ⬇️
cmd/skaffold/app/cmd/dev.go 84.61% <0.00%> (ø)
cmd/skaffold/app/cmd/flags.go 91.00% <0.00%> (+0.18%) ⬆️
cmd/skaffold/app/cmd/render.go 36.66% <0.00%> (-4.72%) ⬇️
cmd/skaffold/skaffold.go 0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/inspect_tests.go 62.50% <14.28%> (-1.14%) ⬇️
cmd/skaffold/app/cmd/lint.go 42.85% <42.85%> (ø)
cmd/skaffold/app/cmd/find_configs.go 48.88% <50.00%> (+0.24%) ⬆️
cmd/skaffold/app/skaffold.go 76.19% <70.00%> (-8.43%) ⬇️
cmd/skaffold/app/cmd/inspect_build_env.go 65.11% <75.00%> (+6.39%) ⬆️
... and 158 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 66df86a...8284714. Read the comment docs.

@halvards
feat(ko): Enable the ko builder in the API
8284714 
Also include ko built artifacts in metadata events.

Tracking: #6041
Obsolete: #6654
@halvards
Copy link
Collaborator Author

halvards commented Nov 9, 2021

Thanks @MarlonGamez, I was a little quick on this one! Have pushed an update.

@halvards halvards added the kokoro:run runs the kokoro jobs on a PR label Nov 9, 2021
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Nov 9, 2021
MarlonGamez
MarlonGamez approved these changes Nov 10, 2021
View reviewed changes
Copy link
Contributor

@MarlonGamez MarlonGamez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ✅ Thanks for addressing the metadata changes

@MarlonGamez MarlonGamez merged commit 471624f into main Nov 10, 2021
@MarlonGamez MarlonGamez deleted the ko-builder-enable-api branch November 10, 2021 01:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MarlonGamez MarlonGamez MarlonGamez approved these changes

@gsquared94 gsquared94 Awaiting requested review from gsquared94 gsquared94 was automatically assigned from GoogleContainerTools/skaffold-team

Assignees
No one assigned
Labels
cla: yes size/XXL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@halvards @MarlonGamez @kokoro-team